-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix picodvi refresh rates (use 60Hz) #10534
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This implements video mode timing changes suggested by @WolfWings in circuitpython issue adafruit#10242. Previously, picodvi was set so that 640x480 and 720x400 used refresh rates faster than 60Hz. These changes lower the refresh rate to 60Hz which should hopefully be compatible with a wider range of HDMI devices.
I tested using a Metro RP2350 and with 10.0.0-alpha.7-1 My 10" dvi/vga display wouldn't display at 720x400. I was using Fruit Jam OS to test and by modifying code.py to configure the screen at 640x480 it did work. After installing this artifact Fruit Jam OS ran without modification at 720x400. edit: I also ran the modified 640x480 Fruit Jam OS on this artifact and it worked as well. |
Is that any better or worse than with 10.0.0-beta.2? |
if these changes make 720x400@60Hz worse for some monitors compared to sticking with 720x400@70Hz, maybe it would be possible to add an additional option to select between 60Hz and 70Hz when initializing the display. How to do that would be a bit beyond my current knowledge though. |
With 10.0.0-beta.2:
So the PR is an improvement, but only on the TV. I am doing this all with |
hmm... I wonder if adjusting |
So... reading a bit about video modes, and comparing that to your monitor specs, it seems that the sync polarity of the current code might be wrong. Currently it has this:
Based on my reading of your specs, some web searching, and some wild guessing, I think |
@WolfWings we would welcome your thoughts about this. |
TL;DR: Sync polarity exists almost only from daisy-chaining so many standards in a row over the decades from the first VGA monitors all the way to the DVI standard that still had that vestigal "technically we can run analog too" support, so I admit I left them all at 0. It's a vestige from the earliest analog CRT days, it was a way to encode 2 extra bits of data for the lowest of the low-end monitors that had dedicated circuits to handle each video mode they supported, the proverbial 'I played this game and my monitor died' days in the era of wacky VGA modes like 256x480 and 360x200 to eek out a tiny bit more performance. I've never personally encountered an instance where fiddling with the sync polarities affected things on digital formats, but I'd certainly say we should try setting the vertical sync polarity to 1 on the DVI-only monitor. That one might be using semi-fixed circuits though and lack a scaler chip to resize input formats, and as a result be extraordinarily picky about the timing. Could you set the CPU clockrate RP2350 as appropriate and see if the 10.0.0-beta.2 WITHOUT this PR works then? Messing with the CPU frequency can impact USB stability and other timing-sensitive things since it's all so interlocked on the RP2350, so set a fixed display image and trigger this using a GPIO pin button maybe then see if it stabilizes? Sticking these lines somewhere convenient on the circuitpython code should do it as I believe everything since... 8.1.0 supports this read/write on RP2040/RP2350? import microcontroller
microcontroller.cpu.frequency = 126000000 # 640x480, 25.2 * 500k
microcontroller.cpu.frequency = 141500000 # 720x400, 28.3 * 500k If those work then the DVI Dell is just exceedingly specific on timing requirement and playing porch-shuffling games and custom timings won't be an option unfortunately as it lacks a scaler chip entirely for sure then. 😭 I am glad the adjusted timings made the picky capture device happy though! |
I have one monitor (my most flexible one) that likes 720x400 via EDID and that is at 70 Hz. I would suggest leaving that resolution at 70 Hz. My Adafruit HDMI capture dongle amazingly likes the existing 720x400 also. Nearly all EDIDs, if they support 640x480, want 60 Hz. My "likes a lot" monitor will take 640x480 at 72 and 75Hz also but a vast majority do not. This has made finding a small (3.5") HDMI panel for a cyberdeck near impossible, finding one that will display >70Hz when the "standard" is 60 Hz at that resolution. My older display with no HDMI but has a large DVI connector likes: 640x480 @60Hz, 800x600 @56Hz, 800x600 @60Hz and some higher resolutions. It hates 640x480 @72Hz and complains on screen. @WolfWings the picoDVI library Adafruit ported was pretty liberal with jacking the CPU frequency when it wanted and there is a warning to not do so oneself if using it ("sit back and look at the pretty pictures" philosophy). It's not ideal but TinyUSB has been written to take into account some frequency changes like 240MHz I believe. I'm also looking for a 400x240 mode (pixel doubled to 800x480) or a straight 800x400 @65 Hz to make my Adafruit 5" HDMI backpack work. It's EDID says that's it's only resolution. I think it can do lower but it hates the 72Hz of current video. I like @samblenny's idea of a frequency parameter if it makes sense. @samblenny @RetiredWizard @danh To test this PR how do I get a UF2? I am eager to have some flexibility without making it too complex for users. PS @FoamyGuy did you complete your EDID additions? |
To find the artifacts, click on one of the builds, above (after opening up the collapsed list), then click "Summary" at the top of the left sidebar and look for "Artifacts" on the page. Here is a direct link: https://github.com/adafruit/circuitpython/actions/runs/16670995112/artifacts/3665739340. You'll then need to unzip it. |
Ok, downloaded to a fresh Rev D FJ, libraries added, code runs 640x480 on the Adafruit HDMI capture dongle. So confirming that 640x480 @ 60 Hz: yes, 320x240 (pixel doubled to 640x480) @ 60 Hz: Yes My 800x480 seems to hate things. But it wants 65 Hz. |
To summarize what I'm reading here about test results so far, it sounds like:
|
720x400 @60Hz works on my workhorse do anything display even though the EDID says it wants 70 Hz I think for changing the two supported resolutions to 60 Hz increases compatibility. Note to @dhalbert - I do get memory allocation errors likely related to a PR you were recently fixing, throwing an error allocating 0 bytes on program startup, Reset button once or twice fixes it. But I believe this is unrelated behavior as I was getting this from older builds also. @ALL - anyone want to add more resolutions? I could make a few suggestions on some pixel doubled higher resolutions. Thanks all for working on this!! |
@WolfWings thanks. I did some testing with the altered CPU speeds. 141_500_000 unfortunately is not a valid CPU frequency. The closest valid ones are This is the datasheet for the video chip used for the U2412M. The newer monitor, Dell S2421HS, works at 720x400 with both Interestingly both display controller chips seem to be pretty old designs. |
What i just posted as #10534 (comment) shows a regression in the PR build if I adjust the CPU frequencies. Maybe that makes sense, |
If the video output or TinyUSB has issues with changing the clock frequency, there should be alerts to this in the documentation. I can see where Adafruit Support gets some angry customer or two saying they needed some specific overclock but X or Y failed to work at "their" frequency. |
I did not have any trouble with the USB connection when I changed the CPU frequency, even to 141MHz. |
Yeah, my hope for this PR is that it can get merged for 10.0.0 to improve the out of box experience for people getting new Fruit Jam boards. I'm hoping this will let people run more of the sample code with the displays they have and cut down on the need for display-related support requests. It would definitely be nice to support even more video modes. But, from what tannewt has said on the subject previously, it sounds like that's a bit of a heavy lift. |
Yes, I have been concerned that 640x480 @ 60 Hz is most common and the folks trying to "make @72 Hz work" would be disappointed. I'm glad you've done this work, thank you. |
Note when this is merged and in a CircuitPython release, some documentation in https://learn.adafruit.com/using-dvi-video-in-circuitpython/overview will need to be changed. That would fall on me. Perhaps some documentation in Read the Docs would need to change also. |
The last bit is expected, as with the PR build you'd be trying to get it to sync on a 50.4Hz refresh rate which is too low for most monitors to accept, though a lot of TVs happily will. Changing the CPU speed is the way to force the 640x480 bitstream as close as possible to 'standard' without playing with custom timings, in theory you could do PLL schenanigans to adjust the HSTS timing similarly but that's a case of "Do you have another clock PLL to spare?" etc, etc. It's kinda shocking how ancient most display panel controllers are, they've been able to do 4K@120Hz and some ridiculous refresh rates on lower resolutions since the 2000's, the limit was always the cables to feed them and some folks even made aftermarket control boards to use multiple cables to feed in parallel. Unfortunately it sounds like the monitor is just incredibly picky with it's display modes, I'd have a bunch of experiments I could do with an older RPi4 or earlier playing with the HDMI settings to toggle between 'true' HDMI with the data islands and 'old' DVI without, etc, but yeah that's unfortunate. |
Minor correction, the changes in the PR are assuming 150Mhz, the stock RP2350 speed. So you're underclocking the CPU setting it to 126Mhz which slews everything else slower as a result. The RP2040 was 125Mhz IIRC? (I kinda skipped that generation so I don't have the specs memorized but I recall the RP2350 was a bit faster.) |
Oops, sorry, yes I forgot that. |
So, considering all that, where do things stand for code review? It sounds like supporting more displays and video modes would be possible but not easy. Are the current changes in this PR good enough? Should I be planning to spend some time in the coming week on making revisions? |
I'll wait for @tannewt to weigh in and we'll probably discuss this internally on Monday. |
I think making these changes as a default would be good. It sounds like it only improves things. I suspect once more people get Fruit Jams we'll need to expose these values to settings.toml. I believe we should be able to patch the hstx command sequences to adjust them. We should require forcing the resolution in that case too. |
These might be able to be tuned a bit more, but we think this is good for now. |
This implements video mode timing changes suggested by @WolfWings in circuitpython issue #10242. Previously, picodvi was set so that 640x480 and 720x400 used refresh rates faster than 60Hz. These changes lower the refresh rate to 60Hz which should hopefully be compatible with a wider range of HDMI devices.
Checks:
This seems to be pretty solid as far as I can tell, but I'd definitely recommend additional testing before merging this.